-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved the handling of cookies. #172
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup. Good work!
@@ -39,7 +39,6 @@ def wrapper(*args, **kwargs): | |||
except RTCException: | |||
raise RTCException("Relogin Failed: " | |||
"Invalid username or password") | |||
kwargs["headers"]["Cookie"] = rtc_obj.headers["Cookie"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here should be kwargs["cookies"] = rtc_obj.cookies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review. I've modified the source code.
@@ -107,23 +114,25 @@ def _get_headers(self): | |||
raise exception.RTCException("Authentication Failed: " | |||
"Invalid username or password") | |||
|
|||
_cookies = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L97 _headers = {"Content-Type": self.CONTENT_XML}
is useless for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review. I've modified the source code.
Please wait to merge. I may have found a bug in this code. |
Sorry. There was a problem with how to set the cookie when not redirecting, so I fixed it. |
@ojima-ryou Please feel free to ping when this PR is ready. Thanks. cc @gpongelli for the cookie handling part, this could be useful for backward compatibility. |
Sure. Before the final review, I suggest to rebase this PR . |
Hello. I've improved rtcclient's use of cookies. Before the improvement, communication with the RTC server was performed while the unnecessary character string was set in the cookie of the HTTP-Header. I think this implementation is more secure. I hope you like it.